On Thu, Aug 4, 2022 at 9:57 AM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Wed, Aug 3, 2022 at 12:11 AM Robert Haas <robertmhaas@gmail.com> wrote: > > > > On Tue, Aug 2, 2022 at 3:25 AM Bharath Rupireddy > > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > ereport_startup_progress infrastructure added by commit 9ce346e [1] > > > will be super-useful for reporting progress of any long-running server > > > operations, not just the startup process operations. For instance, > > > postmaster can use it for reporting progress of temp file and temp > > > relation file removals [2], checkpointer can use it for reporting > > > progress of snapshot or mapping file processing or even WAL file > > > processing and so on. And I'm sure there can be many places in the > > > code where we have while or for loops which can, at times, take a long > > > time to finish and having a log message there would definitely help. > > > > > > Here's an attempt to generalize the ereport_startup_progress > > > infrastructure. The attached v1 patch places the code in elog.c/.h, > > > renames associated functions and variables, something like > > > ereport_startup_progress to ereport_progress, > > > log_startup_progress_interval to log_progress_report_interval and so > > > on. > > > > I'm not averse to reusing this infrastructure in other places, but I > > doubt we'd want all of those places to be controlled by a single GUC, > > especially because that GUC is also the on/off switch for the feature. > > Thanks Robert! How about we tweak the function a bit - > begin_progress_report_phase(int timeout), so that each process can use > their own timeout interval? In this case, do we want to retain > log_startup_progress_interval as-is specific to the startup process? > If yes, other processes might come up with their own GUCs (if they > don't want to use hard-coded timeouts) similar to > log_startup_progress_interval, which isn't the right way IMO. > > I think the notion of ereport_progress feature being disabled when the > timeout is 0, makes sense to me at least. > > On the flip side, what if we just have a single GUC > log_progress_report_interval (as proposed in the v1 patch)? Do we ever > want different processes to emit progress report messages at different > frequencies? Well, I can think of the startup process during standby > recovery needing to emit recovery progress report messages at a much > lower frequency than the startup process during the crash recovery. > Again, controlling the frequencies with different GUCs isn't the way > forward. But we can do something like: process 1 emits messages with a > frequency of 2*log_progress_report_interval, process 2 with a > frequency 4*log_progress_report_interval and so on without needing > additional GUCs. > > Thoughts? Here's v2 patch, passing progress report interval as an input to begin_progress_report_phase() so that the processes can use their own intervals(hard-coded or GUC) if they wish to not use the generic GUC log_progress_report_interval. Thoughts? -- Bharath Rupireddy RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/